Remove process env mutation (#177) (#221) - #223

reviews:
  auto_review:
    drafts: true

_{Comment @coderabbitai help to get the list of available commands.}

Reviewer's Guide

The PR establishes mockable::Env as the workspace boundary for environment-variable access, using DefaultEnv in production and MockEnv in tests; HNSW property tests no longer mutate process-global state, remaining readers are migrated, and Clippy now prevents direct reads or mutations from being reintroduced.

Sequence diagram for injected environment configuration

sequenceDiagram
    participant EntryPoint
    participant Env as mockable::Env
    participant Configuration

    EntryPoint->>Configuration: load_with_env(Env)
    Configuration->>Env: string(variable)
    Env-->>Configuration: configured value or None
    Configuration-->>EntryPoint: resolved configuration

    Note over EntryPoint,Env: Production supplies DefaultEnv, tests supply MockEnv

File-Level Changes

Assessment against linked issues

Possibly linked issues

• Remove the last unsafe env mutation: local ENV_LOCK in hnsw property test search_config.rs #177: The PR directly implements the issue’s proposed HNSW dependency injection and adds the requested Clippy prohibition.
• Remove the last unsafe env mutation: local ENV_LOCK in hnsw property test search_config.rs #177: PR directly implements the issue by injecting environment configuration and deleting the local lock and unsafe mutations.

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Ensure that this is validated against the current version of the codegraph.

If further refinement to address this finding would be deleterious, please supply a clear explanatory one to two paragraph markdown message in a code block that I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced.

String Heavy Function Arguments

chutoro-test-support/src/process.rs:

What lead to degradation?

In this module, 41.7% of all arguments to its 7 functions are strings. The threshold for string arguments is 39.0%

Why does this problem occur?

String is a generic type that fail to capture the constraints of the domain object it represents. In this module, 42 % of all function arguments are string types.

How to fix it?

Heavy string usage indicates a missing domain language. Introduce data types that encapsulate the semantics. For example, a user_name is better represented as a constrained User type rather than a pure string, which could be anything.

#!/bin/bash
set -euo pipefail

printf '%s\n' '== target module outline =='
ast-grep outline chutoro-test-support/src/process.rs --items all || true

printf '%s\n' '== target module =='
cat -n chutoro-test-support/src/process.rs

printf '%s\n' '== process module references and candidate call sites =='
rg -n -C 4 --glob '*.rs' \
  '(?:use\s+.*\bprocess\b|process::|(?:run|run_with|spawn|command|assert).*\()' \
  . || true

printf '%s\n' '== tests and exports around target module =='
rg -n -C 4 --glob '*.rs' \
  '(?:mod\s+process\b|pub\s+(?:use|mod)\s+process\b|#\[(?:test|tokio::test)\])' \
  chutoro-test-support . || true

printf '%s\n' '== workspace package metadata =='
fd -a '^Cargo\.toml$' . -x sh -c 'printf "\n== %s ==\n" "$1"; sed -n "1,160p" "$1"' sh {} \; || true

#!/bin/bash
set -euo pipefail

printf '%s\n' '== chutoro-test-support/src/process.rs =='
cat -n chutoro-test-support/src/process.rs

printf '%s\n' '== public declarations in target module =='
rg -n -C 2 '^\s*pub(?:\([^)]*\))?\s+(?:struct|enum|type|fn|trait|const|static)\b' \
  chutoro-test-support/src/process.rs || true

printf '%s\n' '== imports or qualified references to chutoro_test_support::process =='
rg -n -C 3 --glob '*.rs' \
  '(?:chutoro_test_support::process|use\s+chutoro_test_support::\{?[^;]*\bprocess\b|use\s+chutoro_test_support::process)' \
  . || true

printf '%s\n' '== target-module test declarations =='
rg -n -C 5 '^\s*(?:#\[(?:test|rstest[^\]]*)\]|fn\s+\w+)' \
  chutoro-test-support/src/process.rs || true

@leynos chutoro-test-support/src/process.rs — find_test_binary, find_test_binary_with_env, find_in_deps, is_matching_binary, has_expected_suffix

Suppress this diagnostic. The current codegraph has one public API, find_test_binary(name: &str). Its only consumers are chutoro-test-support/tests/benchmark_regression_gate_cli.rs and chutoro-test-support/tests/kani_nightly_gate_cli.rs. The name arguments identify Cargo [[bin]] targets. The file_name argument is a transient UTF-8 file-system component used only to test the executable suffix.

Do not introduce a TestBinaryName newtype. It would add conversion and forwarding code to a small test-support boundary. It would not add meaningful validation because Cargo defines binary-name validity and the file system defines file names. The current &str parameters accurately support Cargo environment-variable construction, target-path construction, and prefix matching. No AI coding-agent prompt is appropriate because the refactor would be deleterious.

Paste this into the CodeScene diagnostic suppression field:

This module resolves Cargo binary target names and executable file names. The `&str` values are not separate domain entities. They are representations of one externally defined identifier: the Cargo `[[bin]]` name, plus a transient file-name view required for executable-suffix matching.

Introducing a newtype would add conversion and forwarding code without introducing useful validation or preventing invalid states. Cargo owns binary-name semantics, and the file system owns file-name semantics. Keep the small test-support API and its private helpers string-based.

_{You are interacting with an AI system.}